Repository navigation
Breaking-change prototype: compare records and normalize paths safely - #5755
Open
catalinaperalta wants to merge 2 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: eddbfa67-cd77-4acf-99f6-8fd275d6c7d6
catalinaperalta
marked this pull request as ready for review
October 22, 2026 23:11
catalinaperalta
requested review from
Mark Cowlishaw (markcowl),
Timothee Guerin (timotheeguerin) and
Jeff Fisher (xirzec)
as code owners
October 22, 2026 23:11
Mark Cowlishaw (markcowl)
left a comment
Mark Cowlishaw (markcowl)
left a comment
Member
There was a problem hiding this comment.
Looks good, one suggestion
|
|
||
| describe("record comparisons", () => { | ||
| it.each([ | ||
| ["Record<string>", "Record<int32>", "ResourcePropertyTypeChanged"], |
There was a problem hiding this comment.
Should we do the same for Record<string>, Array<string>? Also, do we have a similar test for Array<string>, Array<string> and Array<string>, string[]?
Member
Author
There was a problem hiding this comment.
Added explicit Record ↔ Array cases, Array → Array, and Array ↔ string[] in test/record-diffs.test.ts. Shape changes assert the finding kind and owning property; equivalent array forms assert no findings. Build and all 85 targeted record/diff-engine/operation-identity tests pass. Also merged the latest prototype base and resolved the Azure.Core import conflict. Changes are in 5ee8463.
Timothee Guerin (timotheeguerin)
approved these changes
Oct 23, 2026
Resolve the Azure.Core kind import and add requested Record<string>/Array<string>, unchanged array, and array syntax equivalence cases in both directions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: eddbfa67-cd77-4acf-99f6-8fd275d6c7d6
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Focused fix for #5450. Compares record indexer values and indexed/non-indexed model shapes, distinguishes nested generic instantiations during cycle detection, and preserves the owning property for approvals and source locations. Replaces the path regex with a monotonic scan that preserves embedded placeholders and malformed-path behavior. Addresses discussion_r3984280196, discussion_r3993294760 and discussion_r4099933236 on #5450. Build and 80 targeted engine/identity/record tests pass. Ran pnpm format and pnpm lint; repository lint remains blocked by existing prototype unused declarations, console usage and missing error cause. Unrelated formatting excluded.